Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: compute storage root #1294

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

feat: compute storage root #1294

wants to merge 8 commits into from

Conversation

enitrat
Copy link
Collaborator

@enitrat enitrat commented Jul 22, 2024

Time spent on this PR: tbd

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Resolves #1243

What is the new behavior?


This change is Reviewable

Comment on lines +29 to +38
let base = 0x10;
let bound = 0x10;
let (high, _) = unsigned_div_rem(value, base);
assert [output] = high;
let output = output + 1;
%{
memory[ids.output] = res = (int(ids.value) % PRIME) % ids.base
assert res < ids.bound, f'split_int(): Limb {res} is out of range.'
%}
let output = output + 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just ?

Suggested change
let base = 0x10;
let bound = 0x10;
let (high, _) = unsigned_div_rem(value, base);
assert [output] = high;
let output = output + 1;
%{
memory[ids.output] = res = (int(ids.value) % PRIME) % ids.base
assert res < ids.bound, f'split_int(): Limb {res} is out of range.'
%}
let output = output + 1;
let (high, low) = unsigned_div_rem(value, 0x10);
assert [output] = high;
assert [output + 1] = low;
let output = output + 2;

Comment on lines +40 to +53
let nibbles_len = [fp];
let output_start = cast([fp + 1], felt*);
let count = output - output_start;
let is_done = Helpers.is_zero(nibbles_len - count);
jmp done if is_done != 0;

let next_byte_index = count / 2;
let bytes = cast([fp - 3], felt*);
tempvar value = bytes[next_byte_index];
let range_check_ptr = range_check_ptr + 1;
[ap] = range_check_ptr, ap++;
[ap] = output, ap++;
[ap] = value, ap++;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be simplified to avoid the is_zero calls especially

let prefix = ((2 * is_leaf) + 1) * 16 + self.nibbles[0];
assert encoded[0] = prefix;
tempvar to_pack = new Nibbles(self.nibbles_len - 1, self.nibbles + 1);
let bytes_len = NibblesImpl.pack_nibbles(to_pack, encoded + 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let bytes_len = NibblesImpl.pack_nibbles(to_pack, encoded + 1);
let bytes_len = pack_nibbles(to_pack, encoded + 1);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add docstrings

// Case even number of nibbles
let prefix = 2 * is_leaf * 16;
assert encoded[0] = prefix;
let bytes_len = NibblesImpl.pack_nibbles(self, encoded + 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let bytes_len = NibblesImpl.pack_nibbles(self, encoded + 1);
let bytes_len = pack_nibbles(self, encoded + 1);

return bytes_len;
}

func encode_path{range_check_ptr}(self: Nibbles*, is_leaf: felt) -> (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this is mainly packing the nibbles with a prefix. It gives me the impression that we go from bytes* to nibbles* to bytes* again, not sure to understand

let (local output: felt*) = alloc();

if (nibbles_len == 0) {
tempvar res = new Nibbles(nibbles_len, output);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in done label: new Nibbles(nibbles_len, nibbles);
Maybe standardize the naming of what Nibbles takes by renaming output to nibbles or changing the naming in done label?

from utils.mpt.nibbles import Nibbles, NibblesImpl

func test__from_bytes{range_check_ptr}() -> Nibbles* {
// Given
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to remove if not following given / when / then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: add method to compute storage root
3 participants